Flush transaction log cache in Delta flush_metadata_cache procedure#16466
Flush transaction log cache in Delta flush_metadata_cache procedure#16466ebyhr merged 5 commits intotrinodb:masterfrom
flush_metadata_cache procedure#16466Conversation
ebyhr
changed the title
flush_metadata_cache procedure
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
b5015ef to
aa7008d
Compare
github-actions
Bot
added
delta-lake
There was a problem hiding this comment.
Do we want this procedure installed for iceberg ?
tbh, I didn't know that it existed in iceberg already.
There was a problem hiding this comment.
I'd rather have the method always available in the connector (register it on HiveProcedureModule) and throw an exception (as your implementation does for delta) in case that the caching metastore is not used.
There was a problem hiding this comment.
Added another commit to avoid installing the procedure in Iceberg.
I'd rather have the method always available in the connector
What's the rationale or motivation of this? My opinion is basically the opposite (no need to install a procedure when it's unusable).
There was a problem hiding this comment.
What's the rationale or motivation of this?
I was thinking more from a user perspective where a user may be confused that the procedure is not available (e.g. : while using AWS Glue metastore) for the hive/delta connector even though is advertised to be available in the documentation.
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
aa7008d to
8e50366
Compare
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
2 times, most recently
from
0e7d6ac to
266aec9
Compare
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
266aec9 to
5a09dfb
Compare
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
5a09dfb to
e08137e
Compare
There was a problem hiding this comment.
the flush procedure should be installed whenever caching is supported
what about changing the parameter name and logic.
for example, we could skip binding CachingHiveMetastoreConfig for iceberg (and maybe some others)
There was a problem hiding this comment.
the flush procedure should be installed whenever caching is supported
This makes it hard to exclude Hive's flush_metadata_cache from Delta Lake connector. I initially tried to exclude the procedure from DeltaLakeConnector#getProcedures, but Procedure class doesn't contain catalog or connector information (MethodHandle doesn't work nicely to compare objects).
There was a problem hiding this comment.
would it make sense to try to delegate to Hive's flush procedure?
There was a problem hiding this comment.
I tried to delegate to the connector at first, but the code was a little redundant. Let me merge as-is. I will take another look later.
There was a problem hiding this comment.
do we need a product test?
maybe file: -based tests are enough?
There was a problem hiding this comment.
FileHiveMetastore has the additional cache with listTablesCache #13115. It requires a refactoring to disable the cache (the duration is hard-coded now) or wait until the internal cache is expired.
Additionally, replace toUnmodifiableList with toImmutableList.
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
7892030 to
118e67d
Compare
|
Rebased on master to resolve conflicts. |
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
3 times, most recently
from
c6ae7f8 to
5bb3280
Compare
The procedure was unusable because Iceberg connector always disables caching metastore.
Co-Authored-By: Marius Grama <findinpath@gmail.com>
ebyhr
force-pushed
the
ebi/delta-flush-cache
branch
from
5bb3280 to
3f88633
Compare
4 participants
Description
Relates to #13737
Release notes
(x) Release notes are required, with the following suggested text: